Conversation
Wire the API's new valuableResultPositions field through both feedback commands so agents can attribute usefulness to specific data.web results by 1-indexed position. Skills now surface positions in jq extraction snippets and require exhaustive position marking (unlisted results count as not useful), with valuableSources reserved for URLs outside data.web. Requires an API with position-based search feedback support (firecrawl/firecrawl#4109); the field is only sent when the flag is provided. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Search results come back grouped — data.web, data.images, data.news — and
each group is numbered from 1 independently, so a bare position only ever
meant "web" and could not name a news or image result at all. That matters
most for those groups: news and image results have optional URLs, so
--valuable-sources cannot reliably address them either.
Takes "source:position" pairs (e.g. "web:1,news:2") or a JSON array of
{source, position, reason} entries, matching the API's valuableResults
field (firecrawl/firecrawl#4109). The source is always required — "web:1"
and "news:1" are different results.
Also reverts the generic parsePositiveIntArrayArg helper this branch had
extracted from parsePageNumbersArg; nothing shares it now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dtaivpp
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Conflicts resolved: - skills/firecrawl-cli/SKILL.md: accepted main's deletion. Main folded that skill into skills/firecrawl/ and moved the feedback pattern into skills/firecrawl-search/SKILL.md, which already carries this branch's positional guidance and jq snippet. - skills/firecrawl-search/SKILL.md: kept main's condensed, guarded single-example block and its --missing-content wording; kept this branch's --valuable-results rule bullet, and addressed the returned results in the example by (source, position) rather than URL. valuableResults satisfies the API's substantive-content rule for good and partial, so main's comment on that block stays true. pre-commit hook skipped: pnpm is not on PATH in this environment. Ran the hook's work by hand instead -- `npx lint-staged` (prettier --write over all 38 staged ts/json/md files) reported no modifications. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merge commit's lint-staged run used prettier 3.9.6 from this machine instead of the 3.7.4 the lockfile pins, and 3.9.6 collapses the SetupSubcommand union onto one line. That reformatted a file this branch has no reason to touch, and CI's format:check (on 3.7.4) rejected it. Checked the rest of the branch with `npx prettier@3.7.4 --check` over CI's own globs -- clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
Resolves conflicts from main's Alexandria feedback endpoint and large-result skill guidance. Also enforces --valuable-results as search-only, per review: positions are 1-indexed within a search result group, so sending them on scrape/parse/map produced an API error rather than a clear CLI message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge commit's lint-staged run reformatted src/commands/agent.ts and src/commands/setup.ts with a newer Prettier than the one pinned in pnpm-lock.yaml (3.9.6 vs 3.7.4), which changed how union types wrap and failed the format check. Neither file is part of this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
Review follow-ups on the search skill: The extraction jq walked every key of .data. That predated main's Alexandria matches, so after the merge it emits lines like "tools:1" and "developer:1", and parseValuableResultsArg accepts only web/images/news — an agent following the skill would hand the command input it rejects. Scope it to the three addressable groups, and print "-" for an image or news result with no url so the position is still usable. The worked example hardcoded --valuable-results "web:1,web:3" while telling the reader to replace only the rating. Copied as written, that marks two results useful on every search — false labels in exactly the signal this feature exists to collect. Both fields are placeholders now. Also documents the JSON form of --valuable-results in the README, which only described the compact syntax. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds --valuable-results to search feedback, forwarding valuableResults from the API, with parser, docs, skills, and tests. Changes the CLI public interface and feedback acceptance policy, needing human sign-off.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--valuable-result-positionswith--valuable-resultsonfirecrawl search-feedbackandfirecrawl feedback, forwarding the API'svaluableResultsfield (Collect search feedback on results across all sources firecrawl#4109).source:positionlist ("web:1,news:2") or a JSON array of{source, position, reason}entries.jqsnippets now printsource:positionalongside URLs, and feedback guidance requires exhaustive marking (unlisted results are treated as not useful) with--valuable-sourcesreserved for URLs that were not among the returned results.Why the source is required
Search results come back grouped —
data.web,data.images,data.news— and each group is numbered from 1 independently. A bare position could only ever mean "web", so news and image results were unreachable. That matters most for those groups: theirurlfields are optional, so--valuable-sourcescan't reliably address them either.web:1andnews:1are different results, and the parser rejects a bare1,3rather than guessing.Also
Reverts the generic
parsePositiveIntArrayArghelper this branch had extracted fromparsePageNumbersArg— nothing shares it now that positions aren't a plain int array.Tests
npm run buildnpx vitest run— 23 files, 367 tests passing (5 new parser cases: compact form, JSON form with reasons, missing source, unknown source, non-positive position)🤖 Generated with Claude Code